-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update conformance for EGs #287
Conversation
smpte.js
Outdated
sections.forEach((element) => { | ||
let id = element.id; | ||
if ((id !== "sec-front-matter") && (id !== "sec-foreword") && (id !== "sec-conformance")) { | ||
if (element.innerText.includes("shall")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to handle uppercase and lowercase.
... and what about equivalent forms, e.g., "need to be"? See Clause 7 at https://www.iso.org/sites/directives/current/part2/index.xhtml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to handle uppercase and lowercase.
Addressed in c17ea95
... and what about equivalent forms, e.g., "need to be"? See Clause 7 at https://www.iso.org/sites/directives/current/part2/index.xhtml
I'm using this text from the Conformance
clause we use:
Normative text is text that describes elements of the design that are indispensable or contains the
conformance language keywords: "shall", "should", or "may". Informative text is text that is potentially
helpful to the user, but not indispensable, and can be removed, changed, or added editorially without
affecting interoperability. Informative text does not contain any conformance keywords
I would argue that the keywords are what are usually used, and need to be flagged. Past that it's an editorial thing, and I cannot recall ever seeing any of those equivalents in a SMPTE doc TBH. Also, do EGs docs NEED to conform to ISO guidelines for equivalent forms? I'm just using the OM as a baseline as a first pass.
smpte.js
Outdated
if (docMetadata.pubType === smpte.EG_PUBTYPE) { | ||
|
||
const sections = document.querySelectorAll('section'); | ||
sections.forEach((element) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use for...of
when the body of the loop is huge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tired this the first time around, but the element links don't pass through via the logger_.error
messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should work:
const list = document.querySelectorAll("input[type=checkbox]");
for (const checkbox of list) {
checkbox.checked = true;
}
https://developer.mozilla.org/en-US/docs/Web/API/NodeList#example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is updated in fc85e00
smpte.js
Outdated
const sections = document.querySelectorAll('section'); | ||
sections.forEach((element) => { | ||
let id = element.id; | ||
if ((id !== "sec-front-matter") && (id !== "sec-foreword") && (id !== "sec-conformance")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ((id !== "sec-front-matter") && (id !== "sec-foreword") && (id !== "sec-conformance")) { | |
if (id === "sec-front-matter" || id === "sec-foreword" && id === "sec-conformance") | |
continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic i have catches the sections that are to be excluded from the searching. This update only highlights those sections, and doesn't continue the search past those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic is right. I think the following code style:
while () {
if (bad) continue;
Do a lot of things;
}
is easier to read and maintain than:
while () {
if (not bad) {
Do a lot of things;
}
}
because Do a lot of things;
can be big and you have to remember you are in a particular condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, yea understood. I meant the or
and and
combination. I did the same thing you mention with return;
in the last update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in fc85e00
Change default text of the
Conformance
section for EG docs, and check for conformance language within the doc (outside of the default sections.